Adds zoom fit selected and zoom fit all functionalities. #509
Conversation
Integrate upstream
…o upstream_zoom_fit
Llcoolsouder
left a comment
Llcoolsouder
left a comment
There was a problem hiding this comment.
The previous version of contextMenuEvent allows subclasses to completely customize what shows in the context menu. This change would make it so every subclass gets the zoom actions menu (but only if connected to a DataFlowGraphModel).
It might be more in the spirit to the original to keep the new zoom actions as public methods, but leave them disconnected, allowing end users to connect them where they see fit. (We could have a whole collection of inherited default impl actions that could be useful in this way.)
src/GraphicsView.cpp
Outdated
| bool isZoomFitMenu; | ||
|
|
||
| auto const scenePos = mapToScene(event->pos()); | ||
| if (auto *dfModel = dynamic_cast<DataFlowGraphModel *>(&nodeScene()->graphModel())) { |
There was a problem hiding this comment.
I would caution against coupling the GraphicsView to anything more specific than AbstractGraphModel
There was a problem hiding this comment.
Thanks for the advice! To solve this, a virtual method was defined in AbstractGraphModel and overridden in DataFlowGraphModel, keeping GraphicsView bound to AbstractGraphModel only.
|
|
||
| void GraphicsView::zoomFitAll() | ||
| { | ||
| fitInView(scene()->itemsBoundingRect(), Qt::KeepAspectRatio); |
There was a problem hiding this comment.
This looks like it's supposed to do the same thing as GraphicsView::centerScene. Do we need both?
I've noticed odd behavior in GraphicsView::centerScene. Is zoomFitAll better? (i.e. should this replace the old impl?)
There was a problem hiding this comment.
The central difference between GraphicsView::centerScene and GraphicsView::zoomFitAll is that the former only centralizes the scene without altering the zoom. Since the GraphicsView::centerScene is used to centralize newly loaded scenes I believe both are needed.
src/GraphicsView.cpp
Outdated
| QMenu *menu = nullptr; | ||
|
|
||
| if (!nodeScene()) return; | ||
| bool isZoomFitMenu; |
There was a problem hiding this comment.
This is uninitialized unless all of the immediately following conditions are met.
There was a problem hiding this comment.
The variable is now initialized as false.
src/GraphicsView.cpp
Outdated
|
|
||
| return; |
There was a problem hiding this comment.
| return; |
4 participants
Type of change
Description
The zoom fit selected and zoom fit all features allow for the user to change focus between one, some or all exiting nodes, fitting them in the view accordingly. The video below demonstrates both functionalities:
Zoom_fit_test.mp4
Testing